Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix RuboCop TODOs #476

Merged
merged 2 commits into from
Apr 27, 2022
Merged

Fix RuboCop TODOs #476

merged 2 commits into from
Apr 27, 2022

Conversation

typhoon2099
Copy link
Contributor

Mostly autofixes in here, plus the addition of a super to JWT::JWK::KeyBase.inherited.

@sourcelevel-bot
Copy link

Hello, @typhoon2099! This is your first Pull Request that will be reviewed by SourceLevel, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@sourcelevel-bot
Copy link

SourceLevel has finished reviewing this Pull Request and has found:

  • 5 fixed issues! 🎉

See more details about this review.

@anakinj
Copy link
Member

anakinj commented Apr 27, 2022

I really like this change. I have a tiny-little-supersmall concern about the addition of # frozen_string_literal: true. Did not check that deeply into what kind of strings were used in the files but it could potentially break someones implementation if they for some strange reason were mutating the strings coming out of this gem.

On the other hand the next version will also drop support for older rubies so maybe now is a good time to add that to everywhere.

@excpt excpt merged commit 38039ca into jwt:master Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants